fix(ai): coalesce adjacent user and toolResult turns for Anthropic - #1126

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72f73866e6

Round 1 review

B1. The PR currently fails the required Changelog gate. Running node scripts/check-pr-changelog.mjs --base main on this exact checkout reports runtime source changed without a CHANGELOG.md entry; the new packages/ai/src/changes.md entry only satisfies the separate tracker-coverage policy. Add a packages/ai/CHANGELOG.md Unreleased entry, or apply the no-changelog label if maintainers explicitly classify this as non-user-facing, before merge.

B2. This changes every plain string user turn, not just adjacent turns, from content: string to a one-element content-block array. Because every new user message is now constructed through appendUserBlocks, its lastParam.content string branch is unreachable in the current pipeline, so the old wire representation is never retained. anthropic-messages is shared by the first-party API and multiple Anthropic-compatible backends; this is an unrelated wire-format change with no compatibility coverage in the PR. Preserve the old string shape for standalone string messages and promote only when coalescing, or add explicit coverage and a compatibility decision for every supported backend.

Round-2 review of commit 0c7ee987dcd5bd642fc57ccf670c23495dee564a.

Round-1 blocker resolutions:

• B1 RESOLVED. packages/ai/CHANGELOG.md now has a correctly formatted [Unreleased] / ### Fixed entry, and node scripts/check-pr-changelog.mjs --base main passes with both the release changelog and packages/ai/src/changes.md coverage.
• B2 RESOLVED for the reported standalone-string regression. convertMessages preserves a standalone non-empty string as content: string when no emitted user turn exists to coalesce; it promotes to text blocks only when appending to an existing user turn. I also checked no-cache, system-prompt, empty/whitespace, three-or-more adjacent users, tool-result-only, and interrupted tool-result-plus-user paths. With cache retention enabled, the pre-existing cache-control pass intentionally promotes the final standalone string so it can carry cache_control; the added no-cache compatibility assertion fails if every standalone string regresses to an array. The focused Anthropic compatibility/cache/replay suites passed (55 tests, 4 skipped).

Fresh blocker:

1. packages/ai/test/deferred-tools.test.ts > deferred tools > preserves tool output as sibling content after emitting references fails on this HEAD. The test fixture has user("Hello") -> assistant(tool_use) -> toolResult(s) -> user("Hello"). The new appendUserBlocks correctly treats the final user text as adjacent to the existing serialized user turn containing the tool results, so the received wire content is [tool_result, tool_result, sibling text, image, { type: "text", text: "Hello", cache_control: { type: "ephemeral" } }], while the existing assertion expects only the prior four entries. The extra final text/cache-marker block makes the test fail (the is_error: false fields shown in the diff are not the cause). This is HEAD-only: the test passes at the PR base da38dc52cd37a817db6a7ddacca7cd2f0fe7b546 and at origin/main f97db40079d6a2cb3e1e383586c551e90b6ee049; it fails on HEAD with 1 failed / 25 passed. Update the existing assertion to encode the intended coalesced wire shape, including the final text and marker, and rerun the package suite.

I inspected #1075 as well: it overlaps this same serializer/helper region and carries the cache-checkpoint variant, so any eventual integration must preserve the coalesced content and marker expectations together. The missing @earendil-works/pi-tui/dist/index.js entrypoint is environment-only and was not counted as a PR blocker: direct package resolution fails identically on the PR base, origin/main, and this checkout.

No other new blocker was found.

Round-3 review of commit a2e234eae099d386952a3bf795f1ba9d5c26604f.

Round-2 blocker resolution: VERIFIED. packages/ai/test/deferred-tools.test.ts now includes the fifth coalesced block, { type: "text", text: "Hello", cache_control: { type: "ephemeral" } }. This is the intended wire shape, not only an observed output: convertToolResult emits all tool_result blocks before displaced sibling content, and appendUserBlocks appends the following user text after those existing blocks. The merged #1075 checkpoint test independently codifies the same [tool_result, text-with-cache-control] order for an interrupted tool turn. The standalone-string compatibility and adjacent-turn coverage remain present.

I ran Vitest from the PR worktree against deferred-tools.test.ts, anthropic-adjacent-user-tool-result-coalesce.test.ts, anthropic-provider-native-replay.test.ts, anthropic-tool-reference-integrity.test.ts, and anthropic-final-tool-pair-guard.test.ts: 5 files passed, 50 tests passed.

Fresh blocker - #1075 composition requires a re-merge before approval:

1. Merged #1075 is 54a15cf906d0bcf7933c3065ad743f450f22d449 on origin/main, while this PR remains based on da38dc52cd37a817db6a7ddacca7cd2f0fe7b546. GitHub currently reports this PR as mergeable: CONFLICTING.
2. git merge-tree --write-tree --messages origin/main HEAD reports conflicts in both packages/ai/src/api/anthropic-messages.ts and packages/ai/CHANGELOG.md. The serializer conflict is semantic as well as textual: #1075 adds isToolLoopContinuation, markUserMessageCacheCheckpoint, and the rolling preceding-checkpoint pass, and changes the OAuth identity-marker condition; the current PR head does not contain those changes. Resolving by taking the PR side would drop #1075's checkpoint behavior, while taking the main side requires explicitly preserving this PR's coalesced assertion/test shape.

Please merge/rebase current origin/main into this PR, resolve the serializer and changelog conflicts while retaining both checkpoint logic and coalescing, and rerun the combined Anthropic/deferred-tools coverage. No other new blocker found.

REQUEST_CHANGES

Round-4 adversarial re-review of merged head be7371d09dda035433a1cced0242fe696cf2af08.

Merge-truth verdict:

(a) PASS - convertMessages() emits the assistant tool_use, convertToolResult() appends the tool_result, and the following user string is appended to that same user message. isToolLoopContinuation() then sees the coalesced tail plus preceding assistant tool use, and markUserMessageCacheCheckpoint() marks the final text block (not the tool result).

(b) PASS - ordinary user -> assistant -> user history remains three alternating messages and does not enter the rolling preceding-checkpoint path. With cache retention disabled, both standalone user strings remain content: string; with normal retention, only the normal final-user cache marker promotes the final string.

(c) PASS - three consecutive tool loops retain only the latest tool-result checkpoint and one preceding tool-result checkpoint (second and third loops); the first loop is not marked as part of the rolling pair. The existing checkpoint budget assertion remains satisfied.

(d) PASS - cacheRetention: "none" produces no cache markers, while standalone strings remain strings; coalesced interrupted content remains a block array only where required to represent the combined turn.

Merge-resolution checks: there is one definition each of appendUserBlocks, isToolLoopContinuation, and markUserMessageCacheCheckpoint; all user/tool-result append paths use the unified helper. The final coalesced-and-tool-loop-gated turn gets the marker on its final text block, and deferred-tool ordering remains tool_result blocks, displaced sibling content, then final coalesced text. packages/ai/CHANGELOG.md retains both #1075 checkpoint and #1126 coalescing entries under Unreleased/Fixed.

Verification: targeted Vitest coverage for coalescing, checkpoint stability, deferred tools, provider-native replay, tool-reference integrity, and final-tool-pair guard passed (6 files, 62 tests); LSP diagnostics are clean for the affected source/tests; the changelog gate passes. No new defensible blocker found.

APPROVE